Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrated to qt6 #429

Draft
wants to merge 80 commits into
base: master
Choose a base branch
from
Draft

Conversation

wolfseifert
Copy link
Contributor

I have just migrated a fork of Gittyup to Qt6.

It compiles (without deprecation warnings) and seems to run, but is still mostly untested (on Manjaro-KDE-Linux only).

src/dialogs/CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated
Comment on lines 95 to 122
Qt6
COMPONENTS ${QT_MODULES} LinguistTools
REQUIRED)
if(FLATPAK)
find_package(
Qt5 5.15
Qt6
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have to find a way to support both

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we? What platforms/targets don't have Qt6 on the menu already and/or won't soon? It seems to be a project like this would benefit from staying on top of the toolkit/library game and any effort back-porting to older versions should be on an as-demanded basis and provided by the people that really want to see it happen for whatever reason. The main development effort should be on the latest stable releases when possible and this seems like a great opportunity to hit that target.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supporting both would mean lots of #ifdefs, a nightmare...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rebased the branch, maybe we can go just with Qt6

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@exactly-one-kas do you have anything against it?

@exactly-one-kas exactly-one-kas added the enhancement New feature or request label Jan 27, 2023
@@ -13,8 +13,6 @@
#include <QMessageBox>

int main(int argc, char *argv[]) {
Application::setAttribute(Qt::AA_EnableHighDpiScaling);
Application::setAttribute(Qt::AA_UseHighDpiPixmaps);
Application app(argc, argv, true);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this anymore required?

@@ -505,7 +506,7 @@ class LfsPanel : public QWidget {
watcher->deleteLater();
});

watcher->setFuture(QtConcurrent::run(repo, &git::Repository::lfsTracked));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand why you changed it, but I don't understand why it worked also the other way?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok it does not compile the other way around, but the function definition looks same in Qt5 and Qt6

@@ -61,8 +61,9 @@ DeleteBranchDialog::DeleteBranchDialog(const git::Branch &branch,

entry->setBusy(true);
QStringList refspecs(QString(":%1").arg(upstreamName));
git::Result (git::Remote::*push)(git::Remote::Callbacks*, const QStringList&) = &git::Remote::push;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the benefit of this function pointer?

@Murmele
Copy link
Owner

Murmele commented Oct 8, 2023

@exactly-one-kas do you think we still need win32?

@Murmele
Copy link
Owner

Murmele commented Oct 8, 2023

@exactly-one-kas Can you test and build this branch on Arch Linux?

@Murmele Murmele mentioned this pull request Oct 8, 2023
@Murmele
Copy link
Owner

Murmele commented Oct 27, 2023

Mac: change CustomTheme_mac.mm use float

Win: D:/a/Gittyup/Gittyup/src/index/indexer.cpp:61:3: error: no matching function for call to 'CreateDirectoryW'
CreateDirectory(dir, NULL);

@Murmele
Copy link
Owner

Murmele commented Jan 21, 2024

@probonopd maybe you have an idea why the appimage builder does not find libqxcb.

2024/01/21 11:17:37 Detected Qt 6
2024/01/21 11:17:37 Using $QTDIR: /home/runner/work/Gittyup/Qt/6.5.*/gcc_64
2024/01/21 11:17:37 Looking in /home/runner/work/Gittyup/Qt/6.5.*/gcc_64/plugins
2024/01/21 11:17:37 Could not find 'plugins/platforms/libqxcb.so' in qtPrfxpath, exiting

I printed the paths:

Show QTDIR

bin
doc
include
lib
libexec
metatypes
mkspecs
modules
phrasebooks
plugins
qml
resources
translations

Show QTDIR/plugins

designer
egldeviceintegrations
generic
iconengines
imageformats
networkinformation
platforminputcontexts
platforms
platformthemes
printsupport
qmllint
qmltooling
sqldrivers
tls
wayland-decoration-client
wayland-graphics-integration-client
wayland-shell-integration
xcbglintegrations

Show QTDIR/plugins/platforms

libqeglfs.so
libqlinuxfb.so
libqminimal.so
libqminimalegl.so
libqoffscreen.so
libqvkkhrdisplay.so
libqvnc.so
libqwayland-egl.so
libqwayland-generic.so
libqxcb.so

@Murmele
Copy link
Owner

Murmele commented Jan 21, 2024

Probably I broke MacOs build with 9c349e5..7b12168, but first lets fix Appimage then I can revert the changes

@probonopd
Copy link

@Murmele not really, sorry. Haven't had time to investigate Qt6 yet.

@Murmele
Copy link
Owner

Murmele commented Jan 21, 2024

@Murmele not really, sorry. Haven't had time to investigate Qt6 yet.

Ah thanks, then I will check it out and notify you about

.github/workflows/build.yml Outdated Show resolved Hide resolved
@Murmele
Copy link
Owner

Murmele commented Feb 6, 2024

@probonopd It seems that it was due to the wildcard in the qt version. Just for your information: 00675a7

@Murmele
Copy link
Owner

Murmele commented Feb 26, 2024

We can check the commit c0754a0e061352c8d8d4434952115796063c9401 from GitAhead where the port happened


- version: 5.12.0
check_only: true
- version: 6.5.3
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updating? Maybe it fixes the theme issues?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try it when i have some time this or next week, but i don't think it will fix our problems here, it must have something to do with the custom theme Gittyup has because default ways of theming work in every qt6 version i tried.

@Murmele
Copy link
Owner

Murmele commented May 22, 2024

Then we need to have a deeper look into it because GitAhead does not have this problem with Qt6

- cleanup
- Fix usage of deprecated functions
@0verEngineer
Copy link
Contributor

0verEngineer commented Jun 14, 2024

I just tried to find the differences in GitAheads QT6 commit in order to fix our theming problems.

First things first:

Theme problems on MacOS:

  • highlighting of selected Repo only slightly visible (sshTools is highlighted)
    • This is the same as in the QT5 Gittyup
  • Use Ours and Use Theirs button not colored
    • This is also the same as in the QT5 Gittyup - @Murmele We should open another issue for this as it is confusing

Theme problems on Linux:

  • The theme problems on linux disappered

.git name problem:

@Murmele This has nothing to do with this QT6 migration, we already talked about this somewhere else, am i correct?

@Murmele
Copy link
Owner

Murmele commented Jun 15, 2024

I just tried to find the differences in GitAheads QT6 commit in order to fix our theming problems.

First things first:

Theme problems on MacOS:

* highlighting of selected Repo only slightly visible (sshTools is highlighted)
  
  * This is the same as in the QT5 Gittyup

* Use Ours and Use Theirs button not colored
  
  * This is also the same as in the QT5 Gittyup - @Murmele We should open another issue for this as it is confusing

Theme problems on Linux:

* The theme problems on linux disappered

.git name problem:

@Murmele This has nothing to do with this QT6 migration, we already talked about this somewhere else, am i correct?

You mean #760?

@0verEngineer
Copy link
Contributor

You mean #760?

Yes, but is there already an issue for the "Use Ours", "Use Theirs" button color problem? This happens in Gittyup 1.4.0 on MacOS for me.

@0verEngineer
Copy link
Contributor

@exactly-one-kas If you have some time can you please check out the Windows theming problem this branch has, i am unable to build openssl on windows..

@Murmele
Copy link
Owner

Murmele commented Jun 17, 2024

Currently in Windows it looks like this:
grafik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants